fix(a11y): use figure/figcaption for media block captions#2717
fix(a11y): use figure/figcaption for media block captions#2717nperez0111 merged 2 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR changes file and image block rendering to use semantic ChangesFile & Image Accessibility (semantic figures + caption-aware alt text)
Sequence Diagram(s)sequenceDiagram
participant User
participant FileBlock as File Block
participant WrapperLogic as Wrapper Logic
participant DOM
User->>FileBlock: Add/view file with url & caption
FileBlock->>WrapperLogic: Evaluate url, caption, showLoader
alt url present && caption && not loading
WrapperLogic->>DOM: Render <figure> wrapper
FileBlock->>DOM: Render preview / loader inside <figure>
FileBlock->>DOM: Render <figcaption> with caption
else
WrapperLogic->>DOM: Render <div> wrapper
FileBlock->>DOM: Render preview / loader inside <div>
FileBlock->>DOM: Render caption fallback if applicable
end
DOM-->>User: Display file block
sequenceDiagram
participant User
participant ImageBlock as Image Block
participant AltLogic as Alt Logic
participant DOM
User->>ImageBlock: Render image (with/without caption)
ImageBlock->>AltLogic: Compute alt = name || ""
alt caption present
AltLogic-->>ImageBlock: alt = ""
ImageBlock->>DOM: Render <img alt=""> inside <figure>
ImageBlock->>DOM: Render <figcaption> visible caption
else no caption
AltLogic-->>ImageBlock: alt = name or ""
ImageBlock->>DOM: Render <img alt="name"> (or decorative)
end
DOM-->>User: Accessible output without duplicated announcements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Two CI fixes for #2717: - Block.css: add `margin: 0` to .bn-file-block-content-wrapper so the wrapper looks identical whether it renders as <figure> (captioned) or <div> (uncaptioned). Browser default <figure> margin is `1em 40px`, whereas the previous <div>+<p class="bn-file-caption"> structure had the <p> margins reset by .bn-default-styles. Without this reset the captioned-image visual snapshot grew by ~50px. - server-util/ServerBlockNoteEditor.test.ts.snap: refresh — these snapshots cover the full HTML/markdown round-trip and were missed in the previous snapshot pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
Closes #2055. Supersedes #2056. When a file/image/video/audio block has a caption, render the wrapper as <figure> with a <figcaption> instead of a <div>+<p>. This matches the WCAG-recommended semantic for caption-content association and removes the need for ad-hoc ARIA fallbacks. Image alt text logic is also tightened: - caption present -> alt="" (the figcaption is the accessible name; this avoids screen readers double-announcing the caption) - no caption, name present -> alt={name} - neither -> alt="" (decorative; aria-hidden was dropped because it would have removed unintentionally-unlabeled images from the accessibility tree entirely) Co-Authored-By: Cyril G <c.gromoff@gmail.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two CI fixes for #2717: - Block.css: add `margin: 0` to .bn-file-block-content-wrapper so the wrapper looks identical whether it renders as <figure> (captioned) or <div> (uncaptioned). Browser default <figure> margin is `1em 40px`, whereas the previous <div>+<p class="bn-file-caption"> structure had the <p> margins reset by .bn-default-styles. Without this reset the captioned-image visual snapshot grew by ~50px. - server-util/ServerBlockNoteEditor.test.ts.snap: refresh — these snapshots cover the full HTML/markdown round-trip and were missed in the previous snapshot pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
d520572 to
28991e2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/blocks/Image/block.ts`:
- Around line 119-122: The alt text is set unconditionally which causes
duplicate announcements when a caption/figcaption exists; update the assignments
in imageRender (where image.alt is set) and in imageToExternalHTML (where the
<img> alt attribute is set) to use an empty string if block.props.caption (or
caption) is present — i.e., set alt = "" when block.props.caption is non-empty,
otherwise use block.props.name || ""; keep the existing conditional wrapping
with createFigureWithCaption intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8692e8f2-7aa6-4b5a-b03c-83ac1f19d0f9
⛔ Files ignored due to path filters (28)
packages/server-util/src/context/__snapshots__/ServerBlockNoteEditor.test.ts.snapis excluded by!**/*.snap,!**/__snapshots__/**tests/src/unit/core/clipboard/copy/__snapshots__/text/html/image.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/clipboard/copy/__snapshots__/text/html/nestedImage.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/file/basic.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/file/nested.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/file/noName.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/image.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/image/basic.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/image/nested.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/image/noName.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/image/noPreview.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/image/urlOnly.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/image/withCaption.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/video/withCaption.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/html/image.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/html/image/nested.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/html/image/noName.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/html/image/urlOnly.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/markdown/image.mdis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/markdown/image/urlOnly.mdis excluded by!**/__snapshots__/**tests/src/unit/react/formatConversion/export/__snapshots__/blocknoteHTML/reactFile/basic.htmlis excluded by!**/__snapshots__/**tests/src/unit/react/formatConversion/export/__snapshots__/blocknoteHTML/reactFile/nested.htmlis excluded by!**/__snapshots__/**tests/src/unit/react/formatConversion/export/__snapshots__/blocknoteHTML/reactFile/noName.htmlis excluded by!**/__snapshots__/**tests/src/unit/react/formatConversion/export/__snapshots__/blocknoteHTML/reactImage/basic.htmlis excluded by!**/__snapshots__/**tests/src/unit/react/formatConversion/export/__snapshots__/blocknoteHTML/reactImage/nested.htmlis excluded by!**/__snapshots__/**tests/src/unit/react/formatConversion/export/__snapshots__/blocknoteHTML/reactImage/noName.htmlis excluded by!**/__snapshots__/**tests/src/unit/react/formatConversion/export/__snapshots__/blocknoteHTML/reactImage/noPreview.htmlis excluded by!**/__snapshots__/**tests/src/unit/react/formatConversion/export/__snapshots__/html/reactImage/noName.htmlis excluded by!**/__snapshots__/**
📒 Files selected for processing (5)
packages/core/src/blocks/File/helpers/render/createFileBlockWrapper.tspackages/core/src/blocks/Image/block.tspackages/core/src/editor/Block.csspackages/react/src/blocks/File/helpers/render/FileBlockWrapper.tsxpackages/react/src/blocks/Image/block.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/core/src/editor/Block.css
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/react/src/blocks/File/helpers/render/FileBlockWrapper.tsx
- packages/core/src/blocks/File/helpers/render/createFileBlockWrapper.ts
- packages/react/src/blocks/Image/block.tsx
| // alt describes image content (per WCAG H86); figcaption (when present) | ||
| // is the contextual caption. Fall back to "" so unlabelled images are | ||
| // marked decorative rather than getting a noisy generic fallback. | ||
| image.alt = block.props.name || ""; |
There was a problem hiding this comment.
alt should be empty when a figcaption is present to avoid redundant AT announcements.
The comment correctly notes that a figcaption provides the contextual label, but the code doesn't act on it — image.alt is set to block.props.name regardless of whether a caption (and therefore a figcaption) is also being rendered.
When both block.props.name and block.props.caption are non-empty, screen readers will announce the alt text and the figcaption separately, producing duplicated output. The PR objectives explicitly specify "caption present → alt=""".
The same fix is needed at line 156 inside imageToExternalHTML, where the <img> is likewise assigned block.props.name || "" before being conditionally wrapped in createFigureWithCaption.
♿ Proposed fix (apply in both `imageRender` and `imageToExternalHTML`)
- // alt describes image content (per WCAG H86); figcaption (when present)
- // is the contextual caption. Fall back to "" so unlabelled images are
- // marked decorative rather than getting a noisy generic fallback.
- image.alt = block.props.name || "";
+ // When a figcaption is present it becomes the accessible name for the
+ // figure; keep img alt="" to avoid redundant AT announcements (WCAG H37).
+ // Without a caption, use the file name as the succinct image description,
+ // or "" to mark the image decorative if neither is provided.
+ image.alt = block.props.caption ? "" : block.props.name || ""; image = document.createElement("img");
image.src = block.props.url;
- image.alt = block.props.name || "";
+ image.alt = block.props.caption ? "" : block.props.name || "";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // alt describes image content (per WCAG H86); figcaption (when present) | |
| // is the contextual caption. Fall back to "" so unlabelled images are | |
| // marked decorative rather than getting a noisy generic fallback. | |
| image.alt = block.props.name || ""; | |
| // When a figcaption is present it becomes the accessible name for the | |
| // figure; keep img alt="" to avoid redundant AT announcements (WCAG H37). | |
| // Without a caption, use the file name as the succinct image description, | |
| // or "" to mark the image decorative if neither is provided. | |
| image.alt = block.props.caption ? "" : block.props.name || ""; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/blocks/Image/block.ts` around lines 119 - 122, The alt text
is set unconditionally which causes duplicate announcements when a
caption/figcaption exists; update the assignments in imageRender (where
image.alt is set) and in imageToExternalHTML (where the <img> alt attribute is
set) to use an empty string if block.props.caption (or caption) is present —
i.e., set alt = "" when block.props.caption is non-empty, otherwise use
block.props.name || ""; keep the existing conditional wrapping with
createFigureWithCaption intact.
Summary
Render media blocks (image / video / audio / file) with
<figure>+<figcaption>when a caption is set. Closes #2055, supersedes #2056.Changes
createFileBlockWrapper.ts/FileBlockWrapper.tsx: wrapper element is<figure>when a caption is shown,<div>otherwise; caption uses<figcaption>instead of<p>.Image/block.tsand ReactImage/block.tsx— alt text:alt=""(figcaption is the accessible name; avoids double-announcement)alt={name}alt=""(decorative; no extra ARIA)Video/Audiopreviews: removed interimaria-describedby— figure/figcaption now provides the association.Block.css: resetmargin: 0on.bn-file-block-content-wrapperso the figure version matches the previous div layout (figure defaultmargin: 1em 40pxwas not covered by the existing.bn-default-stylesp-reset).Notes
<figure>+<figcaption>viaparseFigureElement(the export path was already producing that structure).Co-authored with @Ovgodd, whose original PR did the alt-text simplification preserved here.
🤖 Generated with Claude Code
Summary by CodeRabbit
<figure>and<figcaption>elements when captions are present to improve accessibility and document semantics